Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

convert codebase to ESM & add build watch support #377

Closed
wants to merge 29 commits into from
Closed

convert codebase to ESM & add build watch support #377

wants to merge 29 commits into from

Conversation

o-az
Copy link
Contributor

@o-az o-az commented Oct 5, 2023 •
edited
Loading

Convert to ESM & add watch support for build.

Converting to ESM required

  • updating a number of tsconfig settings, including moduleResolution to be Bundler,
  • postfixing local import with .js (in spite of this),
  • replacing __dirname with ESM-safe option,
  • leveraging createRequire for some imports,
  • updating type to module,
  • updating some dependencies to latest (each upgraded dependency had to be upgraded for this to work),
  • small change to logic in core/src/build/handlers.ts#buildRawHandlerFunctions,
  • small change to logic in core/src/config/config.ts#buildConfig,
  • small tweak to vitest config needed for 2 tests (ens & art-gobblers) specifically,
  • import assertions for json imports,
  • switching to default export for pg (and updating the dep),
  • small updates to types here and there,
  • other things I can't remember atm :>

@o-az o-az self-assigned this Oct 5, 2023
@changeset-bot
Copy link

changeset-bot bot commented Oct 5, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: f8876b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 5, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ponder-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 12, 2023 10:52pm

@o-az
Copy link
Contributor Author

o-az commented Oct 6, 2023 •
edited
Loading

@0xOlias all tests pass except for ens and art-gobblers in core. Besides those everything works
all tests are passing. This is ready for review.

https://github.com/0xOlias/ponder/actions/runs/6428097537/job/17454747341?pr=377

@o-az o-az linked an issue Oct 6, 2023 that may be closed by this pull request
[Feature] Convert codebase to ESM #378
Closed
5 tasks
typedarray
typedarray reviewed Oct 6, 2023
View reviewed changes
Copy link
Collaborator

@typedarray typedarray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple small questions. Seems like most of the diff is updating imports to use the .js prefix. Any other areas you were unsure about / I should take a close at?

packages/core/src/build/handlers.ts Outdated
import type { Transaction } from "@/types/transaction.js";
import { isRejected } from "@/utils/promise.js";

const require = createRequire(import.meta.url);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test that hot reloading still works using createRequire with the require cache invalidation trick?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup first thing. Sadly doesn't work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m8 be relevant evanw/esbuild#1828

@o-az
Copy link
Contributor Author

o-az commented Oct 6, 2023

Couple small questions. Seems like most of the diff is updating imports to use the .js prefix. Any other areas you were unsure about / I should take a close at?

If you could search globally for "await import" and review those files that would be great.

Besides that majority of it is adding .js

@o-az o-az changed the title convert codebase to ESM convert codebase to ESM & add build watch support Oct 12, 2023
@kyscott18 kyscott18 closed this Nov 10, 2023
@o-az o-az deleted the o-az/esm-switch branch November 19, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@typedarray typedarray typedarray left review comments

Assignees

@o-az o-az

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature] Convert codebase to ESM
3 participants
@o-az @typedarray @kyscott18